Skip to content

Conversation

@cdiener
Copy link
Member

@cdiener cdiener commented Feb 26, 2025

This brings the method in line with the paper. Still not super fast but it works. Still missing some more tests.

Copy link
Member

@Midnighter Midnighter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mostly checked for style etc. Perhaps @dagl1 could look at the functional side, I'm not into the matter at the moment.

@dagl1
Copy link

dagl1 commented Mar 16, 2025

@Midnighter Ah sorry I didn't realise things required checking.
So output-wise this fastCC version corresponds to the cobra toolbox (matlab) implementation, and both are congruent with checking for blocked reactions (as in, after removing any things that could not carry flux, no more blocked reactions are detected with other methods (one is MACAW from https://www.biorxiv.org/lookup/doi/10.1101/2024.06.24.600481 )
Code wise it looks correct, however I did not do a line by line comparison. Additionally this is all going by the idea that the Matlab implementation is indeed correct. I will run FVA to make sure that the results corroborate (although I am expecting some reactions with very "bad" stoichiometries to show some deviance).

Some small things in regards to the code (but both of you are my seniors so take them as questions)
Edit: I will see if I have time for a full FVA run tomorrow or Tuesday, if that corroborates I think we are good

@Midnighter
Copy link
Member

Thank you for your evaluation @dagl1.

Ah sorry I didn't realise things required checking.

It's not so much that it's strictly required but since most of us work alone rather than pair program, it's always better to have a second pair of eyes.

@cdiener cdiener merged commit b28dbd8 into devel Oct 14, 2025
13 of 14 checks passed
@cdiener cdiener deleted the fix/fastcc branch October 14, 2025 10:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants